Avoid syncing native parent volume to AirPlay protocols#3980
Conversation
MarvinSchenkel
left a comment
MarvinSchenkel
left a comment
There was a problem hiding this comment.
Automated review (no CRITICAL/PROBLEM findings).
The guard in sync_volume_level cleanly addresses the bug: a parent with native volume control reports device/amplifier volume (different scale than AirPlay/RAOP protocol volume), so copying it onto the AirPlay protocol volume on each track change caused unexpected receiver volume changes on devices like Yamaha MusicCast. Docstring and inline comment explain the why, and the two new tests cover both branches.
Thanks @jyundt 🎉
|
I updated my PR message to include the missing PR label and included the test fixes in 22b22a5. @MarvinSchenkel could this also be backported to 2.8.x? |
|
@marcelveldt could you sanity check this? I remember you mentioned airplay forces us to send a volume level when the stream starts. I think this could still result in volume jumps when the receiver uses a different scale. Did you test this with slipping tracks or just playing a different track when something is already playing? |
I reproduced this behavior by slipping (scrubbing?) through an actively playing track and manually advancing to the next track in the queue. |
|
Yes, AirPlay REQUIRES to send the volume, that is part of the protocol. I also notice that the volume differs on some devices, e.g. Sonos, so this most likely means that our scaling just is flawed and we need to fix that instead. This PR is not the right fix as it will fix issue A but causes issue B instead. |
Ah got it, thanks for the clarification. Lemme take another swing at this. |
|
Marking this PR as draft so we can keep track of which PRs needs our attention. Please mark as 'Ready for review' when you want us to have another look 🙏 . |
4 participants
What does this implement/fix?
Avoid copying a protocol-linked parent player's volume into an AirPlay protocol player when the parent has native volume control.
For native receivers/amplifiers, the parent volume represents device volume, while AirPlay/RAOP volume is stream/protocol volume. Some devices, including Yamaha MusicCast receivers, map those scales differently. Reusing the native parent volume as AirPlay startup volume can cause track changes to unexpectedly alter the receiver volume.
Related issue (if applicable):
Types of changes
bugfixnew-featureenhancementnew-providerbreaking-changerefactordocumentationmaintenancecidependenciesTesting
pre-commit run --all-filespasses.pytestpasses, and tests have been added/updated undertests/where applicable.music-assistant/modelsis linked.music-assistant/frontendis linked.Verified with:
python3 -m py_compile music_assistant/providers/airplay/player.py tests/providers/airplay/test_player.pygit diff --checkSKIP=no-commit-to-branch pre-commit run --files music_assistant/providers/airplay/player.py tests/providers/airplay/test_player.pypytest -o addopts="" tests/providers/airplay/test_player.py